-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: task limit improvement #66
feat: task limit improvement #66
Conversation
You also should post QA |
sorry, new here. what do you mean by "post QA"? |
Prove that it works on your org. This would be a link to a github comment |
Test Result: https://github.com/ubiquity-ariesgun/command-start-stop/issues/9#issuecomment-2427166082 |
Currently looking into adjusting the display format: Warning You have reached your max task limit. Please close out some tasks before assigning new ones.
|
Need to specifically handle 404 |
Any specific action when is 404? |
A lot of work is being done on the devpool-directory backend so I anticipate that this file name will eventually be changed or removed so the faster we can diagnose this problem the better. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgot to submit review :D
Should be configurable Also that's wild that I can't delete or hide your review comment |
@0x4007 Why did you hide my comment (if that was you)? |
Your review comment is the exact same content |
@ariesgun, this task has been idle for a while. Please provide an update. |
@0x4007 maybe you are confused because I first posted my comment with the wrong account and then hid it to repost with my own? I can't seem to find twice the same comment from myself. |
Yes that was with the wrong account, I hid that comment. |
316467b
to
32e9280
Compare
Made the issues search configurable: repo, org, or network. Network: Org: Repo: |
@@ -16,6 +53,7 @@ export async function startStopTask(inputs: PluginInputs, env: Env) { | |||
eventName: inputs.eventName, | |||
payload: inputs.eventPayload, | |||
config: inputs.settings, | |||
organizations: [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be here but in the configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reasoning is that it's needed at multiple places so they just put it in the context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec states:
Only check assigned issues within the network, but this should be configurable to either be just within the repo, org, or network
Shouldn't this be configurable? And even if not, putting it in the config also allows to add defaults on decode which would ensure it is properly initialized and types as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. It mentions the scope is configurable: repo, org , or network.. Dont understand what you meant
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe I'm missing something, but the config item assignedIssueScope
can be set to repo
, org
or network
, this configuration
property gets filled based on assignedIssueScope
when the plugin starts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah that's how it behaves.. Based on assignedIssueScope
, it will fill in the organization
accordingly.
src/utils/issue.ts
Outdated
.paginate(context.octokit.rest.search.issuesAndPullRequests, { | ||
q: `org:${payload.repository.owner.login} assignee:${username} is:open is:issue`, | ||
q: `${repoOrgQuery} assignee:${username} is:open is:issue`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you use the organizations with a search
, it is possible that the user has a private profile which will result in a 422
error when querying the search API. You need to provide a fallback for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already a function getAssignedIssuesFallback
isnt it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could reproduce it, but now i am not sure what kind of fallback we can do here??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm, the function getAssignedIssuesFallback
handled it gracefully.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I tested with a user having a private profile, the run crashed on a 422 error. I will test again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the construct to 'await' so it will be able to catch 422 and trigger getAssignedIssuesFallback
Last QA looks ok: Meniole#46 However when using @ariesgun What does |
it is gonna look at devpool-issues.json and look for any assigned issues there |
6a013ba
into
ubiquity-os-marketplace:development
Resolves #64